x-pack/filebeat/input/awss3: aggregate SQS health status reporting#51819
x-pack/filebeat/input/awss3: aggregate SQS health status reporting#51819efd6 wants to merge 2 commits into
Conversation
Replace scattered per-event UpdateStatus calls in the SQS mode with a centralized sqsHealth aggregator. The input now stays Running while making forward progress despite bounded retryable failures, reports Degraded only for sustained conditions needing operator action, filters out context.Canceled from shutdown/reload, and clears Degraded only when the specific causing condition resolves, not when an unrelated poll succeeds. Key changes: - New sqsHealth type (active-condition model with Set*/Clear* + recompute) - Receive errors require 3 consecutive failures before degrading - Transient S3 processing errors no longer degrade (message retries via visibility timeout are by design) - Keepalive receipt-handle failures no longer degrade (message will retry) - Successful message delete+finalize clears disposition conditions - context.Canceled/DeadlineExceeded suppressed at all condition setters - Subsumes StatusReporterHelper dedup with stricter (enum+msg) check This commit covers the legacy SQS path only; v2 path follows separately. Assisted-By: Cursor
🤖 GitHub commentsJust comment with:
|
|
Pinging @elastic/obs-ds-hosted-services (Team:obs-ds-hosted-services) |
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
| defer h.mu.Unlock() | ||
| // Processing errors are transient per the design; the message will | ||
| // retry via visibility timeout. We do not set a condition for individual | ||
| // processing failures; they are logged and counted in metrics. |
There was a problem hiding this comment.
they are logged and counted in metrics.
Actually I think we are missing a metric for this. We only have sqsMessagesReturnedTotal which is per sqs message rather than per s3 object. We really could use a per-object failure counter.
There was a problem hiding this comment.
I haven't added the per-object failure counter. This is probably follow-up work, but I can do it here if you'd like.
There was a problem hiding this comment.
A follow-up is fine. There's another counter that I think would be useful too for the visibility timeout extension failures. We log it today but a counter would be nice.
|
@ishleenk17 could you or someone from obs-ds-hosted-services please prioritise a review on this? We have a customer affected by the SQS health reporting issue in the CrowdStrike FDR integration and want to get it merged quickly. Thank you! |
Proposed commit message
Note
This is PR one of two. This can be backported to branches listed if that path is chosen. I think it should be, but this is open to discussion. The second PR wires the v2 path with the same behaviour and cannot be backported to branches prior to 9.5.
Checklist
stresstest.shscript to run them under stress conditions and race detector to verify their stability../changelog/fragmentsusing the changelog tool.Disruptive User Impact
How to test this PR locally
Related issues
Use cases
Screenshots
Logs